Preserve the root when removeDotSegments pops past it (RFC 3986 §5.2.4) - #190
Open
gaoflow wants to merge 1 commit into
Open
Preserve the root when removeDotSegments pops past it (RFC 3986 §5.2.4)#190gaoflow wants to merge 1 commit into
gaoflow wants to merge 1 commit into
Conversation
UriString::removeDotSegments() used the explode()/array_reduce() shortcut,
where array_pop() also drops the leading empty segment once ".." pops past
the root. So "/../a" returned "a", turning an absolute path into a rootless
one, contrary to RFC 3986 section 5.2.4 which keeps the root ("/a").
The same deviation surfaced through the public helpers Path::withoutDotSegments()
and UriString::normalize() on a scheme-only URI ("foo:/../a" gave "foo:a").
resolve() was already correct, since it re-prepends the slash for URIs with an
authority. BaseUri::removeDotSegments already carried this guard; back-port it
to the canonical implementation.
Member
|
seems the tests are not passing anymore 🤔 |
Author
|
Thanks — confirmed. The failures are in unchanged |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
UriString::removeDotSegments()drops the leading slash when a..segment pops past the root, turning an absolute path into a rootless (relative) one. Per RFC 3986 §5.2.4, popping at the root operates on the (empty) output buffer, so the leading/is preserved.The
explode('/')+array_reduceshortcut represents the leading/as an empty first segment; when..triggersarray_pop, that empty marker is removed along with a real segment, so the root is lost.Because
foo:/../aandfoo:aidentify different resources (absolute vs. rootless path, RFC 3986 §3.3), this is also visible through the public helpers:Fix
BaseUri::removeDotSegments()already carries this exact guard (with the comment "added because some PSR-7 implementations do not respect RFC3986"); it was never back-ported to the canonicalUriStringimplementation. This restores the leading slash only when the input path was absolute:Scope / no regression
resolve()is unchanged: it already re-prepends/for URIs with an authority, so resolution was correct and stays byte-identical. The full RFC 3986 §5.4 resolution corpus (resolveProvider,extraResolutionProvider) still passes.a/../b->b) and non-overshoot paths are untouched (/a/b/../c->/a/c), so no existing behaviour changes.normalize()on a scheme-only URI.